Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: fix zlib tarball generation #32094

Closed
wants to merge 1 commit into from

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Mar 4, 2020

Closes #31858.

Ensures that contrib/optimizations isn't stripped from the source tree to mitigate build failures e.g

../deps/zlib/deflate.c:54:10: fatal error: 'contrib/optimizations/insert_string.h' file not found
#include "contrib/optimizations/insert_string.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I'm not 100% sure we need to include all of contrib/optimizations, so let me know if there are any changes I'll need to make.

cc @MylesBorins @richardlau

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Mar 4, 2020
@MylesBorins
Copy link
Contributor

I'm going through and testing locally. Will add my explicit +1 when I can confirm this is fixed

@codebytere
Copy link
Member Author

codebytere commented Mar 4, 2020

I'd like to fast-track this once we've explicitly confirmed it as well

@MylesBorins MylesBorins added the fast-track PRs that do not need to wait for 48 hours to land. label Mar 4, 2020
@mhart
Copy link
Contributor

mhart commented Mar 4, 2020

Gotta put my two cents in here and say this still looks like a short-term fix.

So long as tarballs are being created after compilation, issues like this will just keep popping up.

Would love it if the build could just be changed so that binaries are built off the tarball files, so the build only passes if the tarball is good.

@nodejs-github-bot
Copy link
Collaborator

@MylesBorins
Copy link
Contributor

I can confirm that this fixes the build problem AND that we need the entire folder... I aggressively tried to save us 92k to no avail.

Copy link
Contributor

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MylesBorins
Copy link
Contributor

Going to go ahead and last this, the failing dgram tests on OSX are known failures

@MylesBorins
Copy link
Contributor

Landed in 2130474

@MylesBorins MylesBorins closed this Mar 4, 2020
MylesBorins pushed a commit that referenced this pull request Mar 4, 2020
PR-URL: #32094
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
@codebytere codebytere deleted the fix-zlib-tarball branch March 4, 2020 21:18
MylesBorins pushed a commit that referenced this pull request Mar 4, 2020
PR-URL: #32094
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Mar 4, 2020
codebytere added a commit that referenced this pull request Mar 15, 2020
PR-URL: #32094
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
codebytere added a commit that referenced this pull request Mar 17, 2020
PR-URL: #32094
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
@codebytere codebytere mentioned this pull request Mar 17, 2020
codebytere added a commit that referenced this pull request Mar 30, 2020
PR-URL: #32094
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. fast-track PRs that do not need to wait for 48 hours to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v13.9.0 tarball seems to be missing deps/zlib/contrib
6 participants